Skip to content

Comments

Lint shell scripts on CI (SQPIT-279)#2310

Closed
supersven wants to merge 16 commits intodevelopfrom
sventennie/shellcheck
Closed

Lint shell scripts on CI (SQPIT-279)#2310
supersven wants to merge 16 commits intodevelopfrom
sventennie/shellcheck

Conversation

@supersven
Copy link
Contributor

@supersven supersven commented Apr 22, 2022 •
edited
Loading

To test it run make shellcheck.

https://wearezeta.atlassian.net/browse/SQPIT-279

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • If this PR changes development workflow or dependencies, they have been A) automated and B) documented under docs/developer/. All efforts have been taken to minimize development setup breakage or slowdown for co-workers.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@supersven supersven temporarily deployed to cachix April 22, 2022 13:06 Inactive
@supersven supersven temporarily deployed to cachix April 22, 2022 13:19 Inactive
@smatting
Copy link
Contributor

smatting commented Apr 26, 2022 •
edited
Loading

If you want to run the task in ci just add the "make shellcheck" line to linters.dhall here https://github.com/zinfra/cailleach/pull/1033

@supersven
Copy link
Contributor Author

supersven commented Apr 26, 2022

If you want to run the task in ci just add the "make shellcheck" line to linters.dhall here https://github.com/zinfra/cailleach/pull/1033

Hey @smatting ,

Thanks for the advice. I will do. But, first I'll have to solve/ignore the lint issues and remove git GitHub action. 😄

stephen-smith
stephen-smith reviewed Apr 26, 2022
View reviewed changes
@supersven supersven force-pushed the sventennie/shellcheck branch from e06ef35 to 749ed06 Compare April 27, 2022 11:08
@supersven supersven temporarily deployed to cachix April 27, 2022 11:08 Inactive
@supersven supersven temporarily deployed to cachix April 27, 2022 11:11 Inactive
@supersven supersven temporarily deployed to cachix April 27, 2022 14:24 Inactive
@supersven supersven temporarily deployed to cachix April 27, 2022 14:59 Inactive
@supersven supersven marked this pull request as ready for review April 27, 2022 15:03
@supersven supersven force-pushed the sventennie/shellcheck branch 2 times, most recently from 4d22a70 to 90c864a Compare April 29, 2022 13:51
@supersven supersven temporarily deployed to cachix April 29, 2022 13:51 Inactive
@supersven supersven temporarily deployed to cachix May 2, 2022 14:27 Inactive
@supersven supersven force-pushed the sventennie/shellcheck branch from 69b44f0 to ebf2a86 Compare May 2, 2022 15:12
@supersven supersven temporarily deployed to cachix May 2, 2022 15:12 Inactive
@supersven supersven temporarily deployed to cachix May 2, 2022 15:12 Inactive
supersven added 6 commits May 3, 2022 07:33
@supersven
Lint shell scripts on CI (SQPIT-279)
52048bd
@supersven
Add some documentation
940780b
@supersven
Fix typo on phony target in Makefile
d025437
@supersven
Fix or mute shellcheck issues
7f0efaa 
To better not break anything: If in doubt, mute the issue. This of course
does not imply that all muted issues are false positives. It just means
that I feel uncomfortable to change these expressions in a way
shellcheck likes. Feel free to fix them!
@supersven
Add changelog
c1f92b7
@supersven
Fix all outstanding shellcheck issues
9eb9fc4
@supersven supersven force-pushed the sventennie/shellcheck branch from ebf2a86 to 9eb9fc4 Compare May 3, 2022 05:34
@supersven supersven temporarily deployed to cachix May 3, 2022 05:34 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 05:34 Inactive
@supersven
Fix: Remove teleprensence
dd14d2b 
It's already included (for Linux) at another place.
@supersven supersven temporarily deployed to cachix May 3, 2022 05:43 Inactive
@supersven supersven force-pushed the sventennie/shellcheck branch from b3a18fb to 9061710 Compare May 3, 2022 09:58
@supersven supersven temporarily deployed to cachix May 3, 2022 09:59 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 09:59 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 10:00 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 10:00 Inactive
@supersven @akshaymankar
Update hack/bin/set-helm-chart-version.sh
c72309c 
Co-authored-by: Akshay Mankar <akshay@wire.com>
@supersven supersven temporarily deployed to cachix May 3, 2022 10:06 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 10:06 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 10:44 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 10:44 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 11:45 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 11:45 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 11:50 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 11:50 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 13:21 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 13:21 Inactive
@supersven supersven temporarily deployed to cachix May 3, 2022 16:44 Inactive
@supersven supersven temporarily deployed to cachix May 4, 2022 09:45 Inactive
@supersven supersven temporarily deployed to cachix May 4, 2022 13:23 Inactive
@supersven supersven mentioned this pull request May 5, 2022
Merged
5 tasks
@supersven
Copy link
Contributor Author

supersven commented May 5, 2022

@akshaymankar , @stephen-smith , I'm very sorry that I wasted your time! But, after spending hours of hunting a subtle bug in nginz-disco.sh, I'm convinced that #2361 is a much saver approach.
I'm fearing that some other subtle quoting bug might appear in a stressful situation... and that could be really painful.

@supersven supersven closed this May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akshaymankar akshaymankar akshaymankar left review comments

+1 more reviewer

@stephen-smith stephen-smith stephen-smith approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@supersven @smatting @stephen-smith @akshaymankar